Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Empty RunNode #2687

Merged
merged 3 commits into from
Oct 15, 2020
Merged

Empty RunNode #2687

merged 3 commits into from
Oct 15, 2020

Conversation

mrBliss
Copy link
Contributor

@mrBliss mrBliss commented Oct 14, 2020

This is done in two steps, see the first two commit messages.

@mrBliss mrBliss added the consensus issues related to ouroboros-consensus label Oct 14, 2020
@mrBliss mrBliss requested a review from edsko October 14, 2020 17:30
Copy link
Contributor

@edsko edsko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent! RunNode finally empty, and the Denegerate instance now empty as well -- the HFC takes care of it all :) Awesome :)

Turn it into the `estimateBlockSize` method of `SerialiseNodeToNodeConstraints`.
Add `estimateHfcBlockSize` to `SerialiseHFC` so that we can provide a default
instance for the HFC which can still be overridden.
Split off the remaining methods from `RunNode` into `NodeInitStorage` in the new
`Ouroboros.Consensus.Node.InitStorage` module.

Introduce (yet) another config: the `StorageConfig`, used by the methods of
`NodeInitStorage`. Reason: we can't project a HFC `TopLevelConfig` to the
`TopLevelConfig` from one era because of the partial consensus and ledger
configs, but we *can* do that for a `StorageConfig`. This makes it possible to
implement `NodeInitStorage` for the HFC.

Since we have an `NodeInitStorage` instance for the HFC, we can give one for
`RunNode` too instead of having to provide it for each HFC instantiation! Our
sin bin is finally empty. End goal achieved 😎
Merge `checkEmpty` and `addBlock` in `addBlockIfEmpty`. This is currently the
only use of this API and merging them expresses the intent better. Moreover,
this avoids the racy nature of the two (although there will be no race condition
in practice).
@mrBliss
Copy link
Contributor Author

mrBliss commented Oct 15, 2020

bors merge

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Oct 15, 2020

@iohk-bors iohk-bors bot merged commit a09f209 into master Oct 15, 2020
@iohk-bors iohk-bors bot deleted the mrBliss/empty-runnode branch October 15, 2020 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consensus issues related to ouroboros-consensus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants